Workaround for GL canonical names without a commit ID and changes for ADR#306
Workaround for GL canonical names without a commit ID and changes for ADR#306NotTheEvilOne merged 3 commits intomainfrom
Conversation
NotTheEvilOne
force-pushed
the
fix/workaround-broken-cnames-302
branch
from
d145fdb to
c8664d2
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #306 +/- ##
==========================================
- Coverage 91.73% 91.68% -0.05%
==========================================
Files 42 42
Lines 2105 2129 +24
==========================================
+ Hits 1931 1952 +21
- Misses 174 177 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
NotTheEvilOne
force-pushed
the
fix/workaround-broken-cnames-302
branch
5 times, most recently
from
0717508 to
ae5d482
Compare
NotTheEvilOne
changed the title
NotTheEvilOne
force-pushed
the
fix/workaround-broken-cnames-302
branch
3 times, most recently
from
949ab91 to
1f66a76
Compare
nkraetzschmar
left a comment
nkraetzschmar
left a comment
There was a problem hiding this comment.
Small bug: the gardenlinux_epoch field in the metadata is incorrectly formatted as a set. (wasn't changed in this PR, but with these fixes I'm now for the first time able to test this locally, so only spotted this now)
Besides that LGTM 🚀
src/gardenlinux/s3/s3_artifacts.py
Outdated
| "build_committish": commit_hash, | ||
| "build_committish": commit_id_or_hash, | ||
| "build_timestamp": datetime.fromtimestamp(release_timestamp).isoformat(), | ||
| "gardenlinux_epoch": {version_epoch}, |
There was a problem hiding this comment.
| "gardenlinux_epoch": {version_epoch}, | |
| "gardenlinux_epoch": version_epoch, |
NotTheEvilOne
force-pushed
the
fix/workaround-broken-cnames-302
branch
2 times, most recently
from
e6dde4e to
ea0581f
Compare
tests/s3/constants.py
Outdated
| # -*- coding: utf-8 -*- | ||
|
|
||
| RELEASE_DATA = """ | ||
| GARDENLINUX_CNAME="container-amd64-1234.1-abc123lo" |
There was a problem hiding this comment.
This does not match the current format of the release file, so for the tests to be meaningful this should probably be changed to container-amd64-1234.1.
This value is of course conditional on what decision we take on gardenlinux/gardenlinux#4267 but until then it might make sense to have the tests validate against an input matching the real release file.
Maybe we can just take an actual release file from the 1877.10 release here instead of a dummy one 🤔
There was a problem hiding this comment.
While the cname is no longer relevant for the S3 artifact upload I changed it back. I would like to keep the dummy file for this PR as changing values here would require additional tests to be changed.
NotTheEvilOne
force-pushed
the
fix/workaround-broken-cnames-302
branch
from
ea0581f to
e207f10
Compare
Fixes: #302 Signed-off-by: Tobias Wolf <wolf@b1-systems.de> On-behalf-of: SAP <tobias.wolf@sap.com>
…names Related: #4268 Signed-off-by: Tobias Wolf <wolf@b1-systems.de> On-behalf-of: SAP <tobias.wolf@sap.com>
…r it Signed-off-by: Tobias Wolf <wolf@b1-systems.de> On-behalf-of: SAP <tobias.wolf@sap.com>
NotTheEvilOne
force-pushed
the
fix/workaround-broken-cnames-302
branch
from
e207f10 to
3431f2d
Compare
2 participants
What this PR does / why we need it:
This PR adds a workaround as we currently have a misalignment between tools handling Garden Linux canonical names. Furthermore it adds some described expectations from ADR "S3 Artifact and Metadata Publishing Contract Between Builder and GLCI" currently implemented in tools utilizing S3 uploads that is under consideration.
Which issue(s) this PR fixes:
Fixes #302
Related gardenlinux/gardenlinux#4268